Skip to content

Conversation

@mrubens
Copy link
Collaborator

@mrubens mrubens commented Jun 9, 2025

Reverts #3772

Looks like this causes issues with saving history, and possibly other things.


Important

Reverts atomic JSON file writes using safeWriteJson, replacing with fs.writeFile, and updates related tests and documentation.

  • Behavior:
    • Reverts use of safeWriteJson for JSON file writes, replacing it with fs.writeFile in modelCache.ts, modelEndpointCache.ts, importExport.ts, FileContextTracker.ts, apiMessages.ts, taskMessages.ts, webviewMessageHandler.ts, and McpHub.ts.
    • Removes atomic write guarantees and locking mechanisms previously provided by safeWriteJson.
  • Tests:
    • Removes safeWriteJson tests from safeWriteJson.test.ts.
    • Updates tests in cache-manager.test.ts and McpHub.test.ts to reflect changes from safeWriteJson to fs.writeFile.
  • Documentation:
    • Deletes use-safeWriteJson.md rule documentation.
  • Dependencies:
    • Removes proper-lockfile and stream-json from package.json dependencies.

This description was created by Ellipsis for d47929e. You can customize this summary. It will automatically update as commits are pushed.

@mrubens mrubens requested review from cte and jr as code owners June 9, 2025 17:47
@dosubot dosubot bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label Jun 9, 2025
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jun 9, 2025
@mrubens mrubens merged commit 8d2eeda into main Jun 9, 2025
14 of 16 checks passed
@mrubens mrubens deleted the revert-3772-use-safe-write-json-for-all-files branch June 9, 2025 17:53
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jun 9, 2025
@github-project-automation github-project-automation bot moved this from Triage to Done in Roo Code Roadmap Jun 9, 2025
@KJ7LNW KJ7LNW mentioned this pull request Jun 15, 2025
@KJ7LNW
Copy link
Contributor

KJ7LNW commented Jun 15, 2025

See also: #4468

daniel-lxs pushed a commit that referenced this pull request Jun 21, 2025
Fix race condition where safeWriteJson would fail with ENOENT errors
during lock acquisition when the parent directory was just created.

The issue occurred when the directory creation hadn't fully synchronized
with the filesystem before attempting to acquire a lock. This happened
primarily when Task.saveApiConversationHistory() called the function
immediately after creating the task directory.

The fix ensures directories exist and are fully synchronized before
lock acquisition by:
- Creating directories with fs.mkdir({ recursive: true })
- Verifying access to created directories
- Setting realpath: false in lock options to allow locking non-existent files

Added comprehensive tests for directory creation capabilities.

Fixes: #4468
See-also: #4471, #3772, #722

Signed-off-by: Eric Wheeler <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer size:XL This PR changes 500-999 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

6 participants